fix: tier 1 correctness and ops fixes (AOF recovery, healthcheck, helm secrets) - #355
Merged
Conversation
- AOF recovery: roll back to snapshot state on mid-file corruption instead of silently keeping a partially-applied (potentially inconsistent) AOF prefix while claiming snapshot-only recovery; add tests for mid-file CRC corruption and the SET-kept/DEL-lost inconsistency case - shard: log the final AOF sync error on shutdown instead of swallowing it - Dockerfile: fix HEALTHCHECK — wget is not installed in the runtime image and the metrics port is disabled by default; use the built-in ember-server --healthcheck instead (same as docker-compose) - helm: stop injecting requirepass as a plaintext env var; store it in a Secret mounted as a file via EMBER_REQUIREPASS_FILE, support existingSecret; set appVersion to 0.4.9, qualify image repository Claude-Session: https://claude.ai/code/session_017uab7k2MyLpWAQwFDNt4tw
kacy
added a commit
that referenced
this pull request
Jul 3, 2026
- AOF recovery: roll back to snapshot state on mid-file corruption instead of silently keeping a partially-applied (potentially inconsistent) AOF prefix while claiming snapshot-only recovery; add tests for mid-file CRC corruption and the SET-kept/DEL-lost inconsistency case - shard: log the final AOF sync error on shutdown instead of swallowing it - Dockerfile: fix HEALTHCHECK — wget is not installed in the runtime image and the metrics port is disabled by default; use the built-in ember-server --healthcheck instead (same as docker-compose) - helm: stop injecting requirepass as a plaintext env var; store it in a Secret mounted as a file via EMBER_REQUIREPASS_FILE, support existingSecret; set appVersion to 0.4.9, qualify image repository
This was referenced Jul 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 1 of a 5-PR cleanup series from a full-codebase audit.
What
recover_shardpreviously kept the partially-applied AOF prefix in the recovered state while logging "using snapshot state only". The kept prefix could be internally inconsistent (a SET applied, a later DEL lost — a state that never existed). Recovery now snapshots the pre-replay state and rolls back to it on error, making the behavior match the log. Two new tests cover mid-file corruption and the inconsistent-prefix case; both fail against the old code.sync()error on clean shutdown was discarded (let _ =); it's now logged as an error since it means writes may be lost.wgetagainst the metrics port) could never pass — wget isn't installed in the runtime image and the metrics port is disabled by default. Switched toember-server --healthcheck(what docker-compose already uses).requirepasswas injected as a plaintext env var visible inkubectl describe. The chart now creates a Secret (or usesember.existingSecret) and mounts it as a file viaEMBER_REQUIREPASS_FILE. Also:appVersionnow tracks the real release (0.4.9) instead oflatest, and the default image repository is fully qualified.Verification
cargo test -p ember-persistence -p emberkv-core --features protobuf: 708 tests pass; the two new recovery tests were confirmed to fail with the fix reverted.cargo clippy -D warningsclean on touched crates.helm lint/helm templatejob; please eyeball the Secret template.Note (pre-existing, unchanged):
--healthchecksends PING, so withrequirepassset the container reports unhealthy — same limitation the docker-compose healthcheck already has. Worth a follow-up if auth'd deployments need docker healthchecks.